-
Notifications
You must be signed in to change notification settings - Fork 4.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: dns cache with empty response #1183
fix: dns cache with empty response #1183
Conversation
c38f4e9
to
ce2e116
Compare
bb36949
to
9670d2f
Compare
Codecov Report
@@ Coverage Diff @@
## master #1183 +/- ##
==========================================
- Coverage 44.82% 44.75% -0.08%
==========================================
Files 485 484 -1
Lines 29494 29477 -17
==========================================
- Hits 13221 13191 -30
- Misses 14872 14884 +12
- Partials 1401 1402 +1
Continue to review full report at Codecov.
|
9670d2f
to
b08d67f
Compare
The empty response without IP addresses does have error code and other messages in it, we need to store it and detect it later. |
This reverts commit 73470e8.
if (option.IPv4Enable && record.A != nil) || (option.IPv6Enable && record.AAAA != nil) { | ||
return nil, dns_feature.ErrEmptyResponse |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found a bug that might caused by this changes. The program may wait until the context timeout if the records are empty, and return this error:
[Info] app/dns: failed to lookup ip for domain api.twitter.com at server DOH//9.9.9.9 > context deadline exceeded
[Info] proxy/dns: ip query > app/dns: returning nil for domain api.twitter.com > multierr: context deadline exceeded |
c.c. @Loyalsoldier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that's my mistake
This reverts commit 73470e8.
this provides a fix that the dns app won't do cache with the IPRecord with a empty address (len(rec.IP == 0)).
extensive review is expected @xiaokangwang